Conversation
src/App.tsx
Outdated
| } | ||
| const store = new Store(); | ||
|
|
||
| function App() { |
There was a problem hiding this comment.
- I'd separate the store from the app.
- Store should only be instantiated in app and not in store file.
| useEffect(() => { | ||
| fetchTasks(hideCompleted).then(setTasks); | ||
| }, [hideCompleted]); | ||
|
|
||
| const addTask = () => { | ||
| setTasks([...tasks, new Task()]) | ||
| }; | ||
| const setAll = async (completed: boolean) => { | ||
| await TasksController.setAll(completed); | ||
| setTasks(await fetchTasks(hideCompleted)); | ||
| } | ||
|
|
||
| store.loadTasks() | ||
| }, [store.hideCompleted]); |
There was a problem hiding this comment.
I'd tie this to the DOM's DOMContentLoaded event like this: document.addEventListener('DOMContentLoaded', store.loadTasks, { once: true }) instead of React.
But I'm also painfully aware that that's not how most people write react apps nowadays. :(
There was a problem hiding this comment.
By using the use effect - I also get the automatic reload on change of hideCompleted - one way is to make the change through a mutation - I get that, but what's the "mobx" way of triggering an action on change of one or more state members?
There was a problem hiding this comment.
The mobx way of adding reactivity to react is via mobx-react bindings. The mobx-react package supports legacy (class based) components + mobx-react-lite, which supports only function components. I'd install only the latter and wrap each component in observer.
There was a problem hiding this comment.
I think that a possible answer to my question (of reloading based on change of hideCompleted) is to use mobx reaction
https://mobx.js.org/reactions.html
There was a problem hiding this comment.
Yeah, you can do that and in fact that's what the mobx-react bindings use. But why would you bother with a lower level and verbose API when you already have a solution: just wrap in observer and you're done.
There was a problem hiding this comment.
Just to make sure that I get you correctly.
Relace the code for loadTasks()
to be:
observer(()=>loadTasksImplementation())?
There was a problem hiding this comment.
No. I'd do the initial load via a DOM event, decoupled from the UI rendering, like in the first message in this thread. This takes care of the bootstrapping phase of the app - it happens once and that's it.
Any other re-rendering as a result of changes to anything that mobx manages is taken care of using props, thanks so mobx-react's observer wrapper for the components which should be reactive.
Co-authored-by: Nick Ribal <[email protected]>
No description provided.